Include DeclaringType.Name for nested types when generating explicit implementation names #692
Conversation
| nameBuilder.Append('.'); | ||
| } | ||
| AppendTypeName(nameBuilder, sourceType); | ||
| AppendTypeName(nameBuilder, sourceType, true); |
There was a problem hiding this comment.
Why not simply call AppendFullTypeName directly instead of going through AppendTypeName using that new parameter?
| else if (ns != null) | ||
| { | ||
| this.name = string.Concat(ns, ".", sourceType.Name, ".", name); | ||
| this.name = string.Concat(ns, ".", GetFullTypeName(sourceType), ".", name); |
There was a problem hiding this comment.
If I remember correctly, the pre-existing code in SwitchToExplicitImplementationName was written the way it is based on the assumption that string.Concat would be faster than instantiating a StringBuilder. The first if clause is therefore the slow path, while the other two are supposedly fast paths.
Your code change introduces StringBuilders in the fast paths, too, so from a performance perspective, they're now all the same and it seems likely that the distinction between three cases no longer needs to be made.
So if we go with your change, that whole method could perhaps be simplified to a single case where a StringBuilder is always instantiated at the start and there are various ifs for the presence/absence of a namespace, for the presence/absence of generic type arguments, etc.
If we go with the existing code's structure, you'd have to find a way of not relying on StringBuilder on the fast paths.
I think I'd favor simplifying the code by rewriting the if conditions to something more obvious, even if it means we no longer have the string.Concat fast paths.
|
@JelleKerkstra, apologies for the very long delay. I realise it has been more than a year since you submitted this PR. If you're no longer interested in this, that's OK, too. Let me know whether or not you'd like to proceed. |
2 participants
This PR resolves the
DynamicProxyExceptionDuplicate element: Castle.DynamicProxy.Generators.MetaMethodwhen generating a proxy that implements the following interfaces due to the nested classes.In the current implementation, only the namespace, type name and method name are considered. In this PR, the declaring type names are also used when generating explicit interface implementations due to matching method names.